repo: Optimize bare-user content object reads a bit

`perf record ostree checkout ...` for a bare-user repo was telling
me we were spending a good 13% of our time in the depchain of `ot_lgexattrat()`.
The problem here is that traversing the `/proc` path turns out to be
somewhat expensive - there are LSM (SELinux) checks, etc.

For regular files, opening and just getting the xattr, then closing is still
quite cheap. For symlinks, we'll always need to open anyways.

This appears to shave about ~0.1 seconds off of a checkout of
`fedora-atomic/25/x86_64/docker-host` on my workstation.

Oh, and this was the last user of `ot_lgexattrat()` so we can kill it, which is
nice - the xattr code should really live in libglnx.

Closes: #796
Approved by: jlebon

diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c
index c8a125439472189df72347dfdcdaf1f0a9468610..bfb9fb37e29c958b80fdf933fb862341f8ec9ff7 100644 (file)
--- a/src/libostree/ostree-repo.c
+++ b/src/libostree/ostree-repo.c
@@ -2770,8 +2770,18 @@ ostree_repo_load_file (OstreeRepo         *self,
               g_autoptr(GBytes) bytes = NULL;
               glnx_fd_close int fd = -1;
 
-              bytes = ot_lgetxattrat (self->objects_dir_fd, loose_path_buf,
-                                      "user.ostreemeta", error);
+              /* In bare-user, symlinks are stored as regular files, so we just
+               * always do an open, then query the user.ostreemeta xattr for
+               * more information.
+               */
+              fd = openat (self->objects_dir_fd, loose_path_buf, O_RDONLY | O_CLOEXEC);
+              if (fd < 0)
+                {
+                  glnx_set_error_from_errno (error);
+                  goto out;
+                }
+
+              bytes = glnx_fgetxattr_bytes (fd, "user.ostreemeta", error);
               if (bytes == NULL)
                 goto out;
 
@@ -2783,21 +2793,6 @@ ostree_repo_load_file (OstreeRepo         *self,
 
               mode = g_file_info_get_attribute_uint32 (ret_file_info, "unix::mode");
 
-              /* Optimize this so that we only open the file if we
-               * need to; symlinks contain their content, and we only
-               * open regular files if the caller has requested an
-               * input stream.
-               */
-              if (S_ISLNK (mode) || out_input)
-                { 
-                  fd = openat (self->objects_dir_fd, loose_path_buf, O_RDONLY | O_CLOEXEC);
-                  if (fd < 0)
-                    {
-                      glnx_set_error_from_errno (error);
-                      goto out;
-                    }
-                }
-
               if (S_ISREG (mode) && out_input)
                 {
                   g_assert (fd != -1);

diff --git a/src/libotutil/ot-fs-utils.c b/src/libotutil/ot-fs-utils.c
index a0a72ca5db5bff07c82116d7754d5f6bbe3ee617..4ecf820bee765e4df1ab3b06290ffa6e067c2c41 100644 (file)
--- a/src/libotutil/ot-fs-utils.c
+++ b/src/libotutil/ot-fs-utils.c
@@ -61,72 +61,6 @@ ot_gopendirat (int             dfd,
   return TRUE;
 }
 
-GBytes *
-ot_lgetxattrat (int            dfd,
-                const char    *path,
-                const char    *attribute,
-                GError       **error)
-{
-  /* A workaround for the lack of lgetxattrat(), thanks to Florian Weimer:
-   * https://mail.gnome.org/archives/ostree-list/2014-February/msg00017.html
-   */
-  g_autofree char *full_path = g_strdup_printf ("/proc/self/fd/%d/%s", dfd, path);
-  GBytes *bytes = NULL;
-  ssize_t bytes_read, real_size;
-  char *buf;
-
-  do
-    bytes_read = lgetxattr (full_path, attribute, NULL, 0);
-  while (G_UNLIKELY (bytes_read < 0 && errno == EINTR));
-  if (G_UNLIKELY (bytes_read < 0))
-    {
-      glnx_set_error_from_errno (error);
-      goto out;
-    }
-
-  buf = g_malloc (bytes_read);
-  do
-    real_size = lgetxattr (full_path, attribute, buf, bytes_read);
-  while (G_UNLIKELY (real_size < 0 && errno == EINTR));
-  if (G_UNLIKELY (real_size < 0))
-    {
-      glnx_set_error_from_errno (error);
-      g_free (buf);
-      goto out;
-    }
-
-  bytes = g_bytes_new_take (buf, real_size);
- out:
-  return bytes;
-}
-
-gboolean
-ot_lsetxattrat (int            dfd,
-                const char    *path,
-                const char    *attribute,
-                const void    *value,
-                gsize          value_size,
-                int            flags,
-                GError       **error)
-{
-  /* A workaround for the lack of lsetxattrat(), thanks to Florian Weimer:
-   * https://mail.gnome.org/archives/ostree-list/2014-February/msg00017.html
-   */
-  g_autofree char *full_path = g_strdup_printf ("/proc/self/fd/%d/%s", dfd, path);
-  int res;
-
-  do
-    res = lsetxattr (full_path, "user.ostreemeta", value, value_size, flags);
-  while (G_UNLIKELY (res == -1 && errno == EINTR));
-  if (G_UNLIKELY (res == -1))
-    {
-      glnx_set_error_from_errno (error);
-      return FALSE;
-    }
-
-  return TRUE;
-}
-
 gboolean
 ot_readlinkat_gfile_info (int             dfd,
                           const char     *path,

diff --git a/src/libotutil/ot-fs-utils.h b/src/libotutil/ot-fs-utils.h
index 13909c8e89d982cc4ad4a3f801910c93643eaa79..edf8b29aeaa1b9c86c9acfffd47a31deaed794b0 100644 (file)
--- a/src/libotutil/ot-fs-utils.h
+++ b/src/libotutil/ot-fs-utils.h
@@ -33,19 +33,6 @@ gboolean ot_gopendirat (int             dfd,
                         int            *out_fd,
                         GError        **error);
 
-GBytes * ot_lgetxattrat (int            dfd,
-                         const char    *path,
-                         const char    *attribute,
-                         GError       **error);
-
-gboolean ot_lsetxattrat (int            dfd,
-                         const char    *path,
-                         const char    *attribute,
-                         const void    *value,
-                         gsize          value_size,
-                         int            flags,
-                         GError       **error);
-
 gboolean ot_readlinkat_gfile_info (int             dfd,
                                    const char     *path,
                                    GFileInfo      *target_info,